Made an Accessor Mixin for getting attachments straight from PhysShip.#1
Open
NythicalNorm wants to merge 1 commit intoblockninja124:1.20.1/mainfrom
Open
Made an Accessor Mixin for getting attachments straight from PhysShip.#1NythicalNorm wants to merge 1 commit intoblockninja124:1.20.1/mainfrom
NythicalNorm wants to merge 1 commit intoblockninja124:1.20.1/mainfrom
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brickyboy showed me the ConcurrentModificationException try catch, I thought I saw that there were references to the attachments stored in PhysShip, so tried to make it work with an Accessor Mixin, so now no need for the sus try catch, right now the belts are not working on the main branch (also MixinBeltBlockEntity has some compile errors), I tested on a old commit and saw that my code does work. if you comment out the errors in MixinBeltBlockEntity, you can see that my code does get the BeltStorageForceInducer attachment from the PhysShip (you could also see that with a breakpoint).
This implementation may be slower than the original implementation if there are a lot of loaded ships. due to this needing to iterate through every attachment on the PhyShip, also I don't know how future-proof this is against changes in VS since this is explicitly not using the API and bypassing it kinda.
An optimization that could be made is caching which ships have the belt attachment on the server ship load event (and at the code where you add the attachment) into a hashmap with the ship id as key, and checking if a ship id is in the hashmap before executing the iteration through all the attachments in the ship, still this should be okay as is.